Skip to content

Conversation

@MathieuDutSik
Copy link
Contributor

Motivation

Recently, the operation find_keys_by_prefix(&[]) has emerged as an operation on stores. This operation
does not work on DynamoDb since the prefix has to be non-empty as well as the key.

Proposal

Add a [1] as prefix.

Test Plan

The CI should catch the bugs. The run_reads has been updated for that.

Release Plan

  • Nothing to do / These changes follow the usual release cycle.
    It is not that important since DynamoDB is not used on TestNet Conway.

Links

None.

@MathieuDutSik MathieuDutSik marked this pull request as ready for review November 3, 2025 15:41
@MathieuDutSik MathieuDutSik requested review from ma2bd and ndr-ds November 3, 2025 15:41
Copy link
Contributor

@ma2bd ma2bd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So in general, value-keys can be empty but not partition-keys?

@MathieuDutSik
Copy link
Contributor Author

So in general, value-keys can be empty but not partition-keys?

The way to deal with empty vs non-empty keys is by prefixing by [1] the key or root_key.

It used to be that we had a root_key that was empty: the infamous &[]. We no longer. So, requiring root_keys to be non-empty is a sensible decision.

There are now contexts where we need to list all the keys of a store and the command is find_keys_by_prefix(&[]). Therefore, it is sensible to allow empty keys and empty prefixes.

@afck
Copy link
Contributor

afck commented Nov 4, 2025

Would an alternative be to simply use a different query (without the AND k >= ? part) if the specified prefix is empty?

@MathieuDutSik
Copy link
Contributor Author

Would an alternative be to simply use a different query (without the AND k >= ? part) if the specified prefix is empty?

I tried that some years ago and as I remember it was not working. But I can retry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants